Move computation of allocator shim contents to cg_ssa #147526
Conversation
|
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in tests/ui/sanitizer cc @rcvalle Some changes occurred in compiler/rustc_codegen_ssa |
rustbot
added
A-LLVM
|
rustbot has assigned @petrochenkov. Use |
|
cc @anforowicz if this gets merged, https://source.chromium.org/chromium/chromium/src/+/main:build/rust/allocator/lib.rs;l=99-107 would need to be removed to avoid a symbol conflict as the definition gets moved from the allocator shim to libstd. cc @jdonszelmann as I believe this will help a bit with externally implementable items. |
This comment has been minimized.
This comment has been minimized.
|
I pushed a fix for the miri failures, but it only shows up at https://github.com/bjorn3/rust/tree/alloc_shim_weak_shape, not on this PR. |
bjorn3
force-pushed
the
alloc_shim_weak_shape
branch
from
98c96d6 to
614496e
Compare
|
The Miri subtree was changed cc @rust-lang/miri |
bjorn3
force-pushed
the
alloc_shim_weak_shape
branch
from
2684fab to
98c96d6
Compare
| !any_dynamic_crate | ||
| } | ||
|
|
||
| pub fn allocator_shim_contents(tcx: TyCtxt<'_>, kind: AllocatorKind) -> Vec<AllocatorMethod> { |
There was a problem hiding this comment.
Instead of replicating this logic in Miri, could we have Miri call this function to figure out what to forward where?
I'm imagining something like the following field in the machine:
/// Indicates which functions should be forwarded to which other functions
/// to emulate what would usually be the allocator shim module during codgen.
allocator_shims: FxHashSet<String, String>,This can then be filled once on startup (so we can use global_fn_name/default_fn_name and it doesn't matter much that they allocate), and when a shim is called we just do a quick lookup.
That is quite different from how we currently handle the ALLOCATOR_METHODS... there we want custom shims for the defaults, not a symbol lookup. This matches the let alloc_attr_flag = match method.name { in codegen_llvm. Given that this is semantically relevant, would be nice to also have the logic shared that marks some of these shims as "magic", and then find a way to use that from Miri.
I guess that's all better done in a future PR, but it seems better than having to duplicate this logic in Miri.
There was a problem hiding this comment.
Yeah, that would be a good idea. And it would become actually necessary to do it that way if externally implementable items were to start using this code as in that case there wouldn't be a fixed set of functions we can hard code in miri.
bjorn3
force-pushed
the
alloc_shim_weak_shape
branch
from
614496e to
238f3d2
Compare
|
Dropped the commit for reworking how |
Currently it is possible to avoid linking the allocator shim when __rust_no_alloc_shim_is_unstable_v2 is defined when linking rlibs directly as some build systems need. However this requires liballoc to be compiled with --cfg no_global_oom_handling, which places huge restrictions on what functions you can call and makes it impossible to use libstd. Or alternatively you have to define __rust_alloc_error_handler and (when using libstd) __rust_alloc_error_handler_should_panic using #[rustc_std_internal_symbol]. With this commit you can either use libstd and define __rust_alloc_error_handler_should_panic or not use libstd and use #[alloc_error_handler] instead. Both options are still unstable though. Eventually the alloc_error_handler may either be removed entirely (though the PR for that has been stale for years now) or we may start using weak symbols for it instead. For the latter case this commit is a prerequisite anyway.
In the future this should make it easier to use weak symbols for the allocator shim on platforms that properly support weak symbols. And it would allow reusing the allocator shim code for handling default implementations of the upcoming externally implementable items feature on platforms that don't properly support weak symbols.
Co-Authored-By: Ralf Jung <post@ralfj.de>
bjorn3
force-pushed
the
alloc_shim_weak_shape
branch
from
238f3d2 to
2e25b58
Compare
| sym::dealloc => CodegenFnAttrFlags::DEALLOCATOR, | ||
| sym::realloc => CodegenFnAttrFlags::REALLOCATOR, | ||
| sym::alloc_zeroed => CodegenFnAttrFlags::ALLOCATOR_ZEROED, | ||
| _ => CodegenFnAttrFlags::empty(), |
There was a problem hiding this comment.
| _ => CodegenFnAttrFlags::empty(), | |
| ALLOC_ERROR_HANDLER => CodegenFnAttrFlags::empty(), |
There was a problem hiding this comment.
alloc, dealloc, realloc and alloc_zeroed are special cases. In the future the allocator shim may have an unbounded set of functions forwarding to default implementations if externally implementable items were to use it.
petrochenkov
added
S-waiting-on-author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
5 participants
In the future this should make it easier to use weak symbols for the allocator shim on platforms that properly support weak symbols. And it would allow reusing the allocator shim code for handling default implementations of the upcoming externally implementable items feature on platforms that don't properly support weak symbols.
In addition to make this possible, the alloc error handler is now handled in a way such that it is possible to avoid using the allocator shim when liballoc is compiled without
no_global_oom_handlingif you use#[alloc_error_handler]. Previously this was only possible if you avoided liballoc entirely or compiled it withno_global_oom_handling. You still need to avoid libstd and to define the symbol that indicates that avoiding the allocator shim is unstable.